Handle invalid versions gracefully for all list formats #13345
Conversation
ichard26
added
type: bug
ichard26
force-pushed
the
bug/handle-invalid-version-list
branch
from
51d6da5 to
ac6533e
Compare
|
Annnnd changing the JSON format to emit the raw version broke our tests. That's enough of a signal to not mess with the format more than necessary. I've updated the PR to only use the raw version as a fallback. |
| try: | ||
| req_string = f"{dist.raw_name}=={dist.version}" | ||
| except InvalidVersion: | ||
| req_string = f"{dist.raw_name}==={dist.raw_version}" |
There was a problem hiding this comment.
The pip list --format=freeze does not produce identical output as pip freeze. For instance direct URLs are not supported. So I'm not sure emitting a === will be what user expect here, but to be honest I don't know why this output format for pip list exists in addition to pip freeze.
There was a problem hiding this comment.
Ah, are you saying that since pip list --format=freeze has never matched the freeze format that people may not expect anything but name==version? That's fair, although I have doubts on how problematic === would be. A) previously, pip list would crash as it had no way of handling invalid versions, and B) the verbose mode already augments the pip list --format=freeze format.
The alternative is that we simply hide the package with broken metadata or let pip continue crashing, and I don't think that's a particularly great outcome. We could warn that a package is hidden due to broken metadata, but given other parts of pip handle invalid versions gracefully, that would be also unexpected IMO.
There was a problem hiding this comment.
are you saying that since
pip list --format=freezehas never matched the freeze format that people may not expect anything butname==version?
Yes, that, but maybe I'm overthinking it.
|
Thanks everyone for the reviews! |
I'm not sure if it's the right call to always emit the raw version in the JSON format, but given that the columns (default) format does emit the raw version, it seemed better to match that. The freeze format is unique because it's meant to be used for reinstallation so normalized versions are preferable. OTOH, JSON is also meant as a machine-readable format, so perhaps we should display the normalized version if possible.I left the normalization intact since changing it broke the tests.Resolves #13341.